Skip to content

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented May 3, 2023

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.

Both GCC and Clang write debuginfo this way for C++ class methods.

Fixes #109730.
Fixes #109934.

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.
@rustbot
Copy link
Collaborator

rustbot commented May 3, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 3, 2023
@michaelwoerister
Copy link
Member

r? @michaelwoerister

@michaelwoerister
Copy link
Member

Thanks for the PR, @cuviper! The change looks good to me.
@bors r+

@bors
Copy link
Collaborator

bors commented May 5, 2023

📌 Commit 10b69dd has been approved by michaelwoerister

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2023
@cuviper
Copy link
Member Author

cuviper commented May 5, 2023

Nominating for 1.70-beta, because in Fedora we ran into the error: Cannot represent a difference across sections building Firefox with Rust 1.69 + LLVM 16, and upstream here we'll be releasing with LLVM 16 in 1.70.0.

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 5, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 6, 2023
…michaelwoerister

debuginfo: split method declaration and definition

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.

Both GCC and Clang write debuginfo this way for C++ class methods.

Fixes rust-lang#109730.
Fixes rust-lang#109934.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 6, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#110577 (Use fulfillment to check `Drop` impl compatibility)
 - rust-lang#110610 (Add Terminator conversion from MIR to SMIR, part #1)
 - rust-lang#110985 (Fix spans in LLVM-generated inline asm errors)
 - rust-lang#110989 (Make the BUG_REPORT_URL configurable by tools )
 - rust-lang#111167 (debuginfo: split method declaration and definition)
 - rust-lang#111230 (add hint for =< as <=)
 - rust-lang#111279 (More robust debug assertions for `Instance::resolve` on built-in traits with non-standard trait items)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f440999 into rust-lang:master May 6, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 6, 2023
@fundawang
Copy link

Will rust 1.69.1 release soon? Because this patch cannot be applied to 1.69.0 tarball without modification.

@cuviper
Copy link
Member Author

cuviper commented May 7, 2023

I wasn't thinking of proposing this for 1.69-stable, since the official release using LLVM 15 didn't suffer that "cannot represent a difference across sections" error. You can use the backport patch I applied in Fedora though:
https://src.fedoraproject.org/rpms/rust/blob/777115da9e2892eb47502c8afcbe2b7742b14879/f/0001-debuginfo-split-method-declaration-and-definition.patch

cuviper added a commit to cuviper/rust that referenced this pull request May 8, 2023
This reverts rust-lang#46722, commit e0ab5d5.

Since rust-lang#111167, commit 10b69dd, we are
generating DWARF subprograms in a way that is meant to be more compatible
with LLVM's expectations, so hopefully we don't need this workaround
rewriting CUs anymore.
@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 11, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2023
Remove the ThinLTO CU hack

This reverts rust-lang#46722, commit e0ab5d5.

Since rust-lang#111167, commit 10b69dd, we are
generating DWARF subprograms in a way that is meant to be more compatible
with LLVM's expectations, so hopefully we don't need this workaround
rewriting CUs anymore.
@cuviper cuviper mentioned this pull request May 20, 2023
@cuviper cuviper modified the milestones: 1.71.0, 1.70.0 May 20, 2023
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 20, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2023
[beta] backport

- debuginfo: split method declaration and definition rust-lang#111167
- Encode VariantIdx so we can decode ADT variants in the right order rust-lang#111494
- Simplify find_width_of_character_at_span. rust-lang#111560

r? cuviper
@cuviper cuviper deleted the type-decl-disubprogram branch May 21, 2023 03:49
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 14, 2023
…isubprogram, r=cuviper

DebugInfo: Updates test cases that add method declarations.

We've investigated one reason why debugging information often goes wrong at https://reviews.llvm.org/D152095.
> LLVM can't handle IR where subprogram definitions are nested within DICompositeType when doing LTO builds, because there's no good way to cross the CU boundary to insert a nested DISubprogram definition in one CU into a type defined in another CU.

In rust-lang#111167, we added a declaration for the DISubprogram for the method. This PR completes this test case.

stream history: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/Dwarf.20CUs/near/384269475.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LTO, alloc and debug compilation problem Cannot represent a difference across sections
7 participants